-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds the description
and show_description
options to Choice
and InquirerControl
#330
Conversation
Hi @pmeier, I know you were also interested in this addition. Feel free to push and update the PR as needed. |
Hi @kiancross, sorry for the delay with the PR. Would you mind approving the CI tests to run? Thanks in advance 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kiancross LMK if you want more tests and if yes what exactly.
@@ -38,6 +38,7 @@ def checkbox( | |||
use_jk_keys: bool = True, | |||
use_emacs_keys: bool = True, | |||
instruction: Optional[str] = None, | |||
show_description: bool = True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@viniciusdc I've changed this to True
by default, because that is what the user expects when they set a description on a Choice
. Plus, since there are no descriptions yet, previous code is not affected by this.
show_selected=True, | ||
show_description=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having both active asserts that they can coexist.
Hi @kiancross, sorry for the constant pings. I just wanted to know if you could have a look into this whenever you have some time 😄 Happy holidays 🎄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Thanks for the PR. Can you force-push the last commit to re-trigger the GitHub actions?
@kiancross Maintainer action is required to run CI: |
CI is running now. But some of the tests are failing. |
questionary/prompts/common.py
Outdated
else: | ||
tokens.pop() # Remove last newline. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else: | |
tokens.pop() # Remove last newline. | |
if not self.show_description and not self.show_selected: | |
tokens.pop() # Remove last newline. |
I think something like this will probably fix the failing tests.
@kiancross could you approve CI again and take another look? Locally, everything is passing for me after 9305d57. |
@kiancross Anything left to do now that CI is passing or can this be merged? |
Merged! Thanks for your work on this :) |
What is the problem that this PR addresses?
closes #269
...
How did you solve it?
I am including a new
show_description
boolean value into theInquirerControl
class to allow a new choice attribute calleddescription
to be displayed at the bottom of the terminal window -- Similar execution as the currentshow_selected
attribute.More details in the linked issue.
...
Checklist